Introduce archive feature for the submissions(in batch actions) by sandeepsajan0 · Pull Request #3003 · HyphaApp/hypha

sandeepsajan0 · 2022-10-11T12:42:20Z

Add archive function to batch action
Make submission tables archive aware, possibility to toggle between all and active (unrachived)
Optional permission so only staff admins can see archived submissions.

codecov · 2022-10-11T13:05:53Z

Codecov Report

Base: 80.45% // Head: 80.35% // Decreases project coverage by -0.09% ⚠️

Coverage data is based on head (f3e4c63) compared to base (b6fde5d).
Patch coverage: 72.45% of modified lines in pull request are covered.

❗ Current head f3e4c63 differs from pull request most recent head 6c8988c. Consider uploading reports for the commit 6c8988c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3003      +/-   ##
==========================================
- Coverage   80.45%   80.35%   -0.10%     
==========================================
  Files         383      384       +1     
  Lines       18559    18718     +159     
==========================================
+ Hits        14931    15041     +110     
- Misses       3628     3677      +49

Impacted Files	Coverage Δ
...ly/funds/templates/funds/submissions_overview.html	`34.69% <0.00%> (-0.73%)`	⬇️
hypha/apply/funds/urls.py	`100.00% <ø> (ø)`
hypha/apply/activity/adapters/activity_feed.py	`81.18% <25.00%> (-2.32%)`	⬇️
hypha/apply/activity/adapters/slack.py	`86.36% <25.00%> (-1.64%)`	⬇️
hypha/apply/users/decorators.py	`76.59% <40.00%> (-4.36%)`	⬇️
hypha/apply/funds/forms.py	`82.40% <53.84%> (-1.20%)`	⬇️
hypha/apply/funds/templatetags/submission_tags.py	`78.78% <58.82%> (-21.22%)`	⬇️
...plates/funds/includes/table_filter_and_search.html	`94.23% <62.50%> (-5.77%)`	⬇️
hypha/apply/funds/permissions.py	`70.00% <66.66%> (-2.73%)`	⬇️
hypha/apply/funds/views.py	`77.13% <77.14%> (-0.10%)`	⬇️
... and 12 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

theskumar · 2022-10-13T05:48:28Z

        MESSAGES.SKIPPED_REPORT: 'handle_skipped_report',
        MESSAGES.REPORT_FREQUENCY_CHANGED: 'handle_report_frequency',
        MESSAGES.BATCH_DELETE_SUBMISSION: 'handle_batch_delete_submission',
+        MESSAGES.BATCH_ARCHIVE_SUBMISSION: 'handle_batch_archive_submission',


Hold onto merging this as #2991 introduces changes in this.

sandeepsajan0 · 2022-10-19T04:09:56Z

/rebased with PR #2991. I think, now we may put it to the test.

frjo · 2022-10-19T12:19:00Z

Merged #2991 so changed base back to main for this PR.

frjo

We hit blocking performance issue on larger datasets with the filter changes.

We have forgotten to add a unarchive batch function (we can do this in a separate PR if easier.)

frjo · 2022-11-14T12:38:55Z


-def get_used_rounds(request):
+def get_used_rounds(request=None, queryset=None):
+    if queryset:


The queryset seems to always have a value.

Also on larger datasets all the "if queryset" versions crash the server with a "Proxy Error".

The queryset seems to always have a value.

Actually, it is a bit tricky part, we are calling it from __init__ and updating the filter's field queryset to a value instead of a callable that's why it is not getting called internally from forms and it always has a queryset.
In the near future, if somebody removes any of the filter queryset update statement, it will get called internally and execute without queryset. That's why I put a check for queryset but if it is taking time, I think we can use separate methods for initialization and to update the filter's field queryset on the basis of the request's queryset.

Also, queryset here has 2 reference, so it is a bit confusing. I'll change request's or view's 'queryset' to 'dataset' just for inside the table.

frjo · 2022-11-22T10:06:38Z

This needs a rebase and migration number update.

sandeepsajan0 · 2022-11-23T05:48:53Z

Rebased. @frjo migration number looks fine to me. Is it conflicting with any migration or PR?

frjo · 2022-11-23T07:55:07Z

Main branch has hypha/apply/funds/migrations/0104_show_deadline.py

And this PR has hypha/apply/funds/migrations/0104_applicationsubmission_is_archive.py

sandeepsajan0 · 2022-11-28T06:06:59Z

@frjo Migrations conflicts have been resolved.

frjo · 2022-11-29T11:04:45Z

@theskumar @sandeepsajan0 There is som kind of issue with the migrations in this PR. I can replicate it with the sandbox db that is in the repo. But if I create a new and empty db I do not get this issue.

Any ideas what the problem is?

$ git switch feature/archive-submissions
$ git pull
$ dropdb sandbox
$ createdb sandbox
$ pg_restore --verbose --clean --if-exists --no-acl --no-owner -d sandbox public/sandbox_db.dump

DATABASES = {
    'default': dj_database_url.config(
        conn_max_age=600,
        default='postgres:///sandbox'
    )
}

$ python manage.py wagtailsiteupdate hypha.test apply.hypha.test 80
$ python manage.py migrate

Trying to access http://apply.hypha.test/dashboard/ gives me this error:

cursor "_django_curs_4518872576_sync_5" does not exist, the cursor id changes but same messages.

In template /Users/frjo/Sites/hypha/hypha/apply/funds/templates/funds/includes/table_filter_and_search.html, error at line 91

sandeepsajan0 · 2022-12-05T05:45:58Z

@frjo Latest changes seem to be working fine for me locally. Let me know if the cursor error is now fixed for you as well.

frjo · 2022-12-05T13:33:14Z

@sandeepsajan0 Seems to be working for me as well. Excellent work! How did you manage to track down the issue?

sandeepsajan0 · 2022-12-06T05:22:12Z

It was just a combination of guesswork and trial & error with the changes. I also couldn't find an explanation yet why 'an extra distinct was causing the cursor issue'. That's why I wasn't sure and asked you if it is working for you as well.

Whatever if it is fixed then we may move forward, I'll do a bit more research on it on weekend, and if I'll find something I'll post it here for the record.
BTW it has something to do with server-side cursor https://docs.djangoproject.com/en/4.1/ref/databases/#server-side-cursors.

sandeepsajan0 · 2022-12-12T12:30:39Z

I found an alignment issue on the test, I think it happened because of css update for the 'show archive' button. I'll fix this and will update the PR.

…admin

…ions, add settings for permissions

… to show/hide archived submissions

…admin

…se flex direction as row with align-items to center

Fixes #2996 - [x] Add archive function to batch action - [x] Make submission tables archive aware, possibility to toggle between all and active (unrachived) - [x] Optional permission so only staff admins can see archived submissions. Co-authored-by: Fredrik Jonsson <frjo@xdeb.org>

sandeepsajan0 requested a review from theskumar October 12, 2022 10:18

theskumar reviewed Oct 13, 2022

View reviewed changes

sandeepsajan0 force-pushed the feature/archive-submissions branch from d80b85d to e09fa80 Compare October 19, 2022 04:07

sandeepsajan0 changed the base branch from main to feature/theskumar/email-notifications October 19, 2022 04:07

frjo changed the base branch from feature/theskumar/email-notifications to main October 19, 2022 12:18

frjo force-pushed the feature/archive-submissions branch from c5d8944 to 8a28e74 Compare November 9, 2022 15:13

sandeepsajan0 force-pushed the feature/archive-submissions branch from ec63d60 to d4b5325 Compare November 14, 2022 03:37

sandeepsajan0 requested review from frjo and theskumar November 14, 2022 04:19

frjo requested changes Nov 14, 2022

View reviewed changes

sandeepsajan0 requested a review from frjo November 17, 2022 04:23

frjo force-pushed the feature/archive-submissions branch from f3e4c63 to 2dd0d01 Compare November 21, 2022 12:06

frjo approved these changes Nov 21, 2022

View reviewed changes

frjo added Type: Feature This is something new (not an enhancement of an existing thing). Type: Minor Minor change, used in release drafter labels Nov 22, 2022

sandeepsajan0 force-pushed the feature/archive-submissions branch from 8e21e92 to 70913cf Compare November 23, 2022 05:45

sandeepsajan0 force-pushed the feature/archive-submissions branch from 70913cf to 6c310d0 Compare November 26, 2022 11:56

frjo force-pushed the feature/archive-submissions branch from cfd9a85 to ca45bd0 Compare December 6, 2022 11:47

frjo added the Status: RTBC Internal Dev use only label Dec 20, 2022

sandeepsajan0 and others added 23 commits January 9, 2023 08:56

Add archive feature to submissions

7b1287e

Update current submissions logic to hide archived submissions from list

b687d09

Add activity's event migrations

2864a68

Add archived submission list view, provide permissions only to staff …

802fff7

…admin

Resolve migrations conflicts with email digest PR

dcfd902

Add active submissions view, update all submissions to active submiss…

f97dd48

…ions, add settings for permissions

Add background color for archived submissions and add a toggle button…

b1afa98

… to show/hide archived submissions

remove archived table and page

5e25acd

fix css lint issues

a16a6af

Update button text, and make archived suitable with filters

2ec6368

Update filter querysets based on archived value

c61e1c8

Add activity's event migrations

b98b8c4

Add archived submission list view, provide permissions only to staff …

a347cb8

…admin

Resolve migrations conflicts with email digest PR

b0c62a6

remove archived table and page

1a63a1e

Refactor the filters' queryset

639b55f

Use separate methods to update filter's queryset

372e0f7

Update UI for show/hide archive link, remove unnecessary margin and u…

77bc511

…se flex direction as row with align-items to center

Remove active submission view, url and template

5ffc524

Resolve migrations conflicts

70c293f

Fix cursor error, by removing unnecessary distinct query

eef7a66

Fix batch action css issue and reviewers filterset issue

5e993f3

Update migrations after rebase.

6c8988c

frjo force-pushed the feature/archive-submissions branch from 2009474 to 6c8988c Compare January 9, 2023 08:01

frjo merged commit 94a7956 into main Jan 9, 2023

frjo deleted the feature/archive-submissions branch February 22, 2023 08:19

Uh oh!

Conversation

sandeepsajan0 commented Oct 11, 2022 • edited by frjo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Oct 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

theskumar Oct 13, 2022

Choose a reason for hiding this comment

Uh oh!

sandeepsajan0 commented Oct 19, 2022

Uh oh!

frjo commented Oct 19, 2022

Uh oh!

frjo left a comment

Choose a reason for hiding this comment

Uh oh!

frjo Nov 14, 2022

Choose a reason for hiding this comment

Uh oh!

sandeepsajan0 Nov 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandeepsajan0 Nov 15, 2022

Choose a reason for hiding this comment

Uh oh!

frjo commented Nov 22, 2022

Uh oh!

sandeepsajan0 commented Nov 23, 2022

Uh oh!

frjo commented Nov 23, 2022

Uh oh!

sandeepsajan0 commented Nov 28, 2022

Uh oh!

frjo commented Nov 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sandeepsajan0 commented Dec 5, 2022

Uh oh!

frjo commented Dec 5, 2022

Uh oh!

sandeepsajan0 commented Dec 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sandeepsajan0 commented Dec 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sandeepsajan0 commented Oct 11, 2022 •

edited by frjo

Loading

codecov Bot commented Oct 11, 2022 •

edited

Loading

sandeepsajan0 Nov 15, 2022 •

edited

Loading

frjo commented Nov 29, 2022 •

edited

Loading

sandeepsajan0 commented Dec 6, 2022 •

edited

Loading